@llvm/pr-subscribers-backend-x86

Author: Peter Collingbourne (pcc)

diff --git a/llvm/lib/CodeGen/MachineFunction.cpp b/llvm/lib/CodeGen/MachineFunction.cpp
index 38ad582ba923c..429a17a9113d3 100644
--- a/llvm/lib/CodeGen/MachineFunction.cpp
+++ b/llvm/lib/CodeGen/MachineFunction.cpp
@@ -211,9 +211,8 @@ void MachineFunction::init() {
   ConstantPool = new (Allocator) MachineConstantPool(getDataLayout());
   Alignment = STI->getTargetLowering()->getMinFunctionAlignment();
 
-  // FIXME: Shouldn't use pref alignment if explicit alignment is set on F.
   // FIXME: Use Function::hasOptSize().
-  if (!F.hasFnAttribute(Attribute::OptimizeForSize))
+  if (!F.getAlign() && !F.hasFnAttribute(Attribute::OptimizeForSize))
     Alignment = std::max(Alignment,
                          STI->getTargetLowering()->getPrefFunctionAlignment());
 
diff --git a/llvm/test/CodeGen/X86/function-align.ll b/llvm/test/CodeGen/X86/function-align.ll
new file mode 100644
index 0000000000000..11d0e99929927
--- /dev/null
+++ b/llvm/test/CodeGen/X86/function-align.ll
@@ -0,0 +1,18 @@
+; RUN: llc -function-sections < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK: .section .text.f1
+; CHECK-NOT: .p2align
+; CHECK: f1:
+define void @f1() align 1 {
+  ret void
+}
+
+; CHECK: .section .text.f2
+; CHECK-NEXT: .globl f2
+; CHECK-NEXT: .p2align 1
+define void @f2() align 2 {
+  ret void
+}

This has a side-effect you might not want.

Due to the way member function pointers work in the Itanium ABI, all member functions must have at least 2 byte alignment. clang represents this by marking each member function "align 2". This used to allow increasing the alignment if it wasn't specified, but with this patch, all member functions have exactly 2-byte alignment.

Interesting. I had not considered that consequence.

I think longer term we will want a way for frontends to query the subtarget's minimum and preferred alignment. Then here clang could query the right one based on whether it's optimizing for size and set the align attribute to the max of that and 2.

Maybe for now we can introduce a min-align attribute and have clang set it to 2 on member functions.

The alignment is queried directly by optimizations through getPointerAlignment; I don't think we can decrease it after the fact.

I'd suggest adding an attribute in the opposite direction; for functions where we don't want the alignment to increase, add a function attribute "minimize-alignment", and the backend won't increase alignment for those functions.

Oh, also, on the subject of function alignment, we currently still have

The alignment is queried directly by optimizations through getPointerAlignment; I don't think we can decrease it after the fact.

The idea is that min-align would only be capable of increasing the function alignment, not decreasing it. For example, given a function with min-align 2, on x86 with -Os we would end up with alignment 2 and x86 without -Os would have alignment 16, and on aarch64 the alignment would still always be at least 4.